This won't compile — Config.BundleConfig.setTrigger(...) doesn't exist. The diff adds the Config.Trigger enum but no trigger field, setTrigger, or getTrigger on BundleConfig (grep trigger in Config.java finds only the enum). Same problem at line 100 with plugin.getBundle(...).getTrigger() — BundleDownloader/Bundle has setTrigger but no getTrigger. Both test methods fail to compile, so the whole opa-services test source set won't build.

The configured trigger is never propagated, so MANUAL has no effect. This builder chain sets service/resource/polling but never calls .setTrigger(bundleConfig.getTrigger()), so the downloader keeps its default PERIODIC and the new MANUAL branch in startPolling is unreachable via config. (It also can't, yet — BundleConfig.getTrigger() doesn't exist.) Even once it compiles, initialize_manualTrigger_setsTrigger would fail because the bundle stays PERIODIC.

The validation this asserts doesn't exist. BundlePlugin.validate() has no check that rejects polling when the trigger is manual (no trigger reference in the method), so no error containing "cannot specify polling when trigger is manual" is ever produced and this assertion fails. The "reject polling configuration when using the manual trigger" change from the PR summary is missing from the diff.

This reverts the max_size_bytes feature that's on main — likely a stale-branch/bad-merge artifact. The diff deletes BundleConfig.maxSizeBytes (the field, getMaxSizeBytes/setMaxSizeBytes public API, and its Javadoc), drops the bundle.setMaxSizeBytes(...) wiring in BundlePlugin, and switches to the 2-arg new TarballBundleLoader(name, bundleData) (BundlePlugin.java:186). On main this is a configurable DoS safeguard (512 MB default, capping both the compressed download and decompressed contents). Net effect: the configurable limit is gone (falls back to the loader's built-in default) and this line silently changes the default 512 MB → 1 GB. This is unrelated to the trigger feature and shouldn't land; please rebase on current main.

refresh() is broken for repeated use and blocks the caller. It calls downloadBundle() synchronously on the caller's thread (every other download runs on the scheduler, doing blocking network I/O), and returns initialActivation — a one-shot CompletableFuture whose completion sites are all guarded by if (!initialActivation.isDone()). So the returned future reflects only the first activation: after the first success it's already completed, and later refresh() calls return an already-done future that doesn't track that refresh's outcome (success or failure). Also, in MANUAL mode startPolling returns initialActivation without ever scheduling a download, so getInitialActivation() never completes until a refresh() occurs — which can hang plugin startup if lifecycle code awaits it. Consider returning a fresh future per refresh and dispatching the download on the scheduler.

Thanks for the review! I've addressed the compile issues, trigger propagation, validation, and rebased onto the latest main. For the refresh() redesign, I understand the goal of returning a fresh future per refresh while keeping initialActivation for the first activation only. Before I refactor it, could you clarify the expected behavior for manual mode? Should startPolling() return an already-completed initialActivation in manual mode, or should initialActivation only complete after the first successful refresh()?

I checked how OPA's Go bundle plugin handles this:

1. Manual mode does no download at startup — the downloader's Start() is a no-op unless the trigger is periodic; a fetch only happens on Trigger().
2. Readiness = a bundle actually activated — the plugin stays NotReady until every bundle has activated once. But OPA's Start() never blocks on this; readiness is just reported status.

So no, startPolling() shouldn't return a pre-completed initialActivation. Keep it meaningful (first successful activation). The reason a never-completing future hangs us is SDK-specific: build() blocks up to 60s for bundles to reach OK, and BundlePlugin.start() couples that status to allOf(initialActivation). The fix is to decouple them, like OPA:

• start() shouldn't gate the plugin's OK on a manual bundle's initialActivation — loaders are started and awaiting triggers, so the plugin reaches OK and build() won't hang.
• initialActivation completes only on the first successful refresh — don't complete it eagerly.
• refresh() returns a fresh future per call, dispatched on the scheduler (not synchronously on the caller's thread).

A caller wanting to block until the first manual bundle loads awaits refresh(), not getInitialActivation().